feat(ledger): integrate Ledger v3 operator#490
Open
flemzord wants to merge 31 commits into
Open
Conversation
Introduce a new GatewayGRPCAPI custom resource that allows modules to expose gRPC services through the gateway, following the same pattern as GatewayHTTPAPI. gRPC routing uses fully-qualified protobuf service names (e.g. formance.ledger.v1.LedgerService) instead of HTTP path prefixes. Traffic is served on the same port (8080) with h2c protocol enabled conditionally when gRPC APIs are registered. Each GatewayGRPCAPI creates a dedicated <name>-grpc Kubernetes Service pointing to the module's gRPC port, avoiding conflicts with existing HTTP services. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The deletion step was timing out on K8s 1.34 because the script timeout (2m) was being consumed by kubectl wait --for=delete. Increase the script timeout to 5m and the wait to 3m for more headroom. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract the operator image build from Earthly into a standard Dockerfile so it can be used by both Earthly (via FROM DOCKERFILE) and the new Pulumi deployment app. The Pulumi app (deployment/operator/) builds the operator image, applies CRDs, and deploys the operator via its Helm chart. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Helm release creates the namespace itself via createNamespace. Defaults to formance-system. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Docker build context may not include .git, causing go build to fail with "error obtaining VCS status". Disable VCS stamping since version info is already passed via ldflags. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (4)
WalkthroughThe change adds Ledger v3 reconciliation and configuration, gRPC gateway APIs and routing, backend TLS support, multi-stage container packaging, Pulumi deployment automation, network policies, documentation, and extensive unit and integration coverage. ChangesLedger v3 and gateway integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🛑 Changes requested — automated reviewAll previously raised issues (security, deployment, networking, documentation, and logic correctness) appear to have been resolved and marked as such in the prior discussion. One new finding remains from this review round: the GatewayGRPCAPI reconciler lacks the admission guard that GatewayHTTPAPI already has for backendRef name collisions. When a GatewayGRPCAPI's spec.backendRef.name matches the conventional managed Service name (<spec.name>-grpc), the reconciler deletes that Service and the Caddy route then points at a nonexistent backend, silently breaking gRPC routing on every reconcile. Adding the same kind of collision check already present for the HTTP API would fix this. |
NumaryBot
reviewed
Jul 15, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 2 new inline findings.
Summary: #490 (comment)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (1)
internal/tests/networkpolicy_controller_test.go (1)
100-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd slice length assertions for consistency and safety.
While Gomega safely catches panics within
Eventuallyblocks, it is better practice to explicitly assert the lengths ofIngressandFrombefore indexing them. This provides clearer error messages upon failure and matches the pattern used in theallow-ledger-v3-clustercheck above.♻️ Proposed refactor
// Existing preview clusters keep their historical immutable selector. Eventually(func(g Gomega) { np := &networkingv1.NetworkPolicy{} g.Expect(LoadResource(stack.Name, "allow-ledger-v3-preview-cluster", np)).To(Succeed()) g.Expect(np).To(BeControlledBy(stack)) g.Expect(np.Spec.PodSelector.MatchLabels).To(HaveKeyWithValue("formance.com/ledger-v3-preview", "true")) + g.Expect(np.Spec.Ingress).To(HaveLen(1)) + g.Expect(np.Spec.Ingress[0].From).To(HaveLen(1)) g.Expect(np.Spec.Ingress[0].From[0].PodSelector.MatchLabels).To(Equal(np.Spec.PodSelector.MatchLabels)) }).Should(Succeed())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tests/networkpolicy_controller_test.go` around lines 100 - 107, Add explicit length assertions in the existing-cluster check within the Eventually block before indexing np.Spec.Ingress and np.Spec.Ingress[0].From. Assert each collection has the expected element count, then retain the existing selector equality assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deployment/operator/.gitignore`:
- Line 1: Update the ignore pattern in the deployment/operator .gitignore so it
targets the bin directory relative to that file, ensuring
deployment/operator/bin/ is ignored.
In `@deployment/operator/helpers.go`:
- Around line 94-106: Update the platform selection logic around cfg.Get("arch")
so an unset architecture leaves all entries in allPlatforms unchanged,
preserving both supported platforms. Apply the HasSuffix filtering and
linux-architecture fallback only when arch is explicitly configured, rather than
defaulting it to amd64.
- Around line 159-164: Update the CacheToRegistryArgs construction in the
CacheTo configuration so Ref contains only the registry image name and tag,
removing the ",mode=max" suffix. Set the typed Mode field on CacheToRegistryArgs
to "max" instead.
In `@deployment/operator/main.go`:
- Around line 36-42: Validate the result of filepath.Glob in the CRD loading
flow before declaring crds, and return a descriptive error when no manifest
paths are discovered. Preserve the existing glob-error handling and continue
processing the discovered files normally.
- Around line 64-76: Update the licence configuration in the main Pulumi setup
to retrieve licence-token with config.GetSecret(ctx, "licence-token") instead of
cfg.Get, and pass that secret output through unchanged when assigning the
"token" Helm value so it remains secret in the Pulumi graph and state. Keep the
existing issuer fallback and other licenceValues entries unchanged.
In `@Dockerfile`:
- Around line 28-34: Update the final Docker image stage after copying the
operator binary to create an unprivileged user and configure the image to run as
that user, ensuring the existing ENTRYPOINT continues to invoke
/usr/bin/operator without root privileges.
In `@docs/04-Modules/03-Ledger.md`:
- Around line 7-8: Update the PostgreSQL and Broker Markdown links in the ledger
documentation to use descriptive link text such as “PostgreSQL configuration”
and “message broker configuration” instead of “here,” while preserving their
existing destinations.
- Around line 10-12: Update the Ledger v3 requirements documentation to
explicitly list cert-manager as a required dependency, including its Issuer and
Certificate CRDs. Keep the existing Ledger Operator and native-storage
requirements, and ensure the guidance applies consistently to all referenced v3
deployment sections.
In `@docs/09-Configuration` reference/02-Custom Resource Definitions.md:
- Line 2559: Fix the `ClusterSpec` reference in the Cluster table by either
generating the missing `ClusterSpec` schema heading and fragment target or
removing the link while retaining the type text. Ensure the rendered
documentation no longer points to the nonexistent `#clusterspec` anchor.
- Around line 2385-2387: Remove the duplicate, undescribed ready status rows
from both Gateway status tables in the documentation source, retaining the
documented ready entry with its description. Regenerate the generated reference
file so both affected sections contain only one ready row.
In `@Earthfile`:
- Around line 48-54: Declare the `tag` build argument before the `FROM
DOCKERFILE` statement that passes `$tag` as the `VERSION` build argument. Keep
the existing `LICENCE_PUBLIC_KEY_B64` and `EARTHLY_BUILD_SHA` declarations and
forwarding unchanged.
In `@internal/resources/gateways/Caddyfile.gotpl`:
- Around line 152-155: Update the health-check probe URLs in the Caddyfile
template branches around HealthCheckBackend so they use https:// when
HealthCheckBackend.TLS is configured and http:// otherwise. Apply the backend’s
configured CA trust settings to these probes, preserving the existing host,
port, and endpoint paths for both default and custom health checks.
In `@internal/resources/ledgers/init.go`:
- Around line 51-65: Update the Ledger downgrade logic around
ledgerV3ClusterAvailable and getV3Cluster to distinguish confirmed v3 CRD
absence from discovery, list, or watch failures. When Cluster access is unknown,
preserve the migration guard and stop v2 reconciliation instead of treating the
cluster as absent; only proceed with the current v2 path after confirmed absence
or successful existence validation.
In `@internal/resources/ledgers/v3_preview.go`:
- Around line 143-159: Update the preview cleanup flow around the cert-manager
availability guard to find and delete the core TLS Secret independently, before
returning when ledgerV3CertManagerAvailable is false. Ensure deletion is
restricted by the Secret’s preview label or equivalent ownership check, preserve
not-found handling, and avoid making Certificate cleanup a prerequisite; adjust
TestDeleteLedgerV3PreviewSkipsCertManagerResourcesWhenUnavailable to assert the
Secret is removed while cert-manager resources remain untouched.
In `@internal/resources/ledgers/v3_spec.go`:
- Around line 142-143: Update the configuration mapping for ReadKeySetMaxRetries
in the spec construction to validate the int value against math.MaxInt32 before
converting it to int32; only assign spec.Auth.ReadKeySetMaxRetries when it is
nonzero and within range, preventing overflow into a negative retry count.
In `@internal/tests/gateway_controller_test.go`:
- Around line 418-421: Update the Eventually assertion around LoadResource to
require that
deployment.Spec.Template.Annotations["formance.com/backend-tls-secrets-hash"] is
both different from initialHash and non-empty, preserving the existing rollout
wait while rejecting a removed or missing annotation.
---
Nitpick comments:
In `@internal/tests/networkpolicy_controller_test.go`:
- Around line 100-107: Add explicit length assertions in the existing-cluster
check within the Eventually block before indexing np.Spec.Ingress and
np.Spec.Ingress[0].From. Assert each collection has the expected element count,
then retain the existing selector equality assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 51707418-0707-455a-91e5-004d025d7d96
⛔ Files ignored due to path filters (41)
config/crd/bases/formance.com_gatewaygrpcapis.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_gatewayhttpapis.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_gateways.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_ledgerconfigurations.yamlis excluded by!**/*.yamlconfig/crd/kustomization.yamlis excluded by!**/*.yamlconfig/rbac/ledgerconfiguration_editor_role.yamlis excluded by!**/*.yamlconfig/rbac/ledgerconfiguration_viewer_role.yamlis excluded by!**/*.yamlconfig/rbac/role.yamlis excluded by!**/*.yamlconfig/samples/formance.com_v1beta1_ledgerconfiguration.yamlis excluded by!**/*.yamlconfig/samples/kustomization.yamlis excluded by!**/*.yamldeployment/operator/Pulumi.yamlis excluded by!**/*.yamldeployment/operator/go.modis excluded by!**/*.moddeployment/operator/go.sumis excluded by!**/*.sum,!**/*.sumdocs/09-Configuration reference/settings.catalog.jsonis excluded by!**/*.jsongo.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gatewaygrpcapis.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gatewayhttpapis.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gateways.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_ledgerconfigurations.formance.com.yamlis excluded by!**/*.yamlhelm/operator/templates/gen/rbac.authorization.k8s.io_v1_clusterrole_formance-manager-role.yamlis excluded by!**/gen/**,!**/*.yaml,!**/gen/**internal/tests/crds/cert-manager.io_certificates.yamlis excluded by!**/*.yamlinternal/tests/crds/cert-manager.io_issuers.yamlis excluded by!**/*.yamlinternal/tests/crds/ledger.formance.com_clusters.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-audit.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-ledger-and-another-service.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-ledger-and-grpc.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-ledger-only.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-opentelemetry.yamlis excluded by!**/*.yamltests/e2e/chainsaw/02-stack-lifecycle/asserts/networkpolicies.yamlis excluded by!**/*.yamltests/e2e/chainsaw/14-ledger-module/chainsaw-test.yamlis excluded by!**/*.yamltests/e2e/chainsaw/14-ledger-module/resources/database.yamlis excluded by!**/*.yamltests/e2e/chainsaw/14-ledger-module/resources/stack.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/chainsaw-test.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/gateway.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/grpcapi-updated.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/grpcapi.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/httpapi-ledger.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/stack.yamlis excluded by!**/*.yamltools/kubectl-stacks/go.modis excluded by!**/*.modtools/kubectl-stacks/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (50)
.gitignoreDockerfileEarthfilePROJECTapi/formance.com/v1beta1/gateway_types.goapi/formance.com/v1beta1/gatewaybackend_types.goapi/formance.com/v1beta1/gatewaygrpcapi_types.goapi/formance.com/v1beta1/gatewayhttpapi_types.goapi/formance.com/v1beta1/ledger_types.goapi/formance.com/v1beta1/ledgerconfiguration_types.goapi/formance.com/v1beta1/zz_generated.deepcopy.godeployment/operator/.gitignoredeployment/operator/helpers.godeployment/operator/main.godocs/04-Modules/03-Ledger.mddocs/09-Configuration reference/01-Settings.mddocs/09-Configuration reference/02-Custom Resource Definitions.mdinternal/core/setup.gointernal/resources/all.gointernal/resources/auths/env.gointernal/resources/gatewaygrpcapis/create.gointernal/resources/gatewaygrpcapis/init.gointernal/resources/gatewayhttpapis/create.gointernal/resources/gateways/Caddyfile.gotplinternal/resources/gateways/caddyfile.gointernal/resources/gateways/caddyfile_test.gointernal/resources/gateways/configuration.gointernal/resources/gateways/deployment.gointernal/resources/gateways/init.gointernal/resources/ledgers/init.gointernal/resources/ledgers/v3.gointernal/resources/ledgers/v3_preview.gointernal/resources/ledgers/v3_spec.gointernal/resources/ledgers/v3_spec_test.gointernal/resources/ledgers/v3_test.gointernal/resources/ledgers/v3_tls.gointernal/resources/settings/opentelemetry.gointernal/resources/stacks/networkpolicies.gointernal/tests/application_test.gointernal/tests/auth_scopes_settings_test.gointernal/tests/gateway_controller_test.gointernal/tests/gatewaygrpcapi_controller_test.gointernal/tests/jobs_controller_test.gointernal/tests/ledger_controller_test.gointernal/tests/ledger_v3_controller_test.gointernal/tests/networkpolicy_controller_test.gointernal/tests/orchestration_controller_test.gointernal/tests/registries_test.gointernal/tests/transactionplane_controller_test.gointernal/tests/wallets_controller_test.go
`ledgerV3TLSName` returned `<stack>-tls`, the same Secret name the gateway ingress already uses for its own TLS certificate (`internal/resources/gateways/ingress.go` names it `<gateway>-tls`, and the gateway is named after the stack). When gateway ingress TLS is enabled, this duplicates an existing mechanism: two independent cert-manager clients target the *same* Secret — - the ledger v3 `Certificate` (self-signed, per-stack Issuer), and - the gateway ingress (ingress-shim, public ClusterIssuer) Each observes the other's issuance as `IncorrectIssuer` and reissues, producing an unbounded reissue race: the keypair rotates every few seconds, the ledger raft mTLS cert reloader repeatedly fails with `tls: private key does not match public key`, and the raft cluster never forms quorum (stuck Degraded). Rename the raft TLS Secret to `<stack>-ledger-v3-tls` so it no longer collides with the gateway ingress Secret. The gateway consumes this backend certificate by the `GatewayBackendTLSSecretLabel` label (not by name), so a dedicated name is safe. `ledgerV3TLSName` is the single source used by the Certificate/secretName, the ledger pod mount, the gateway backend ref and the preview cleanup, so the rename propagates consistently. This also matches the name already expected in `v3_spec_test.go` (`stack0-ledger-v3-tls`). Co-authored-by: David Ragot <david@formance.com>
NumaryBot
reviewed
Jul 16, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 13 new inline findings.
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 15 new inline findings.
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 14 new inline findings.
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 16 new inline findings.
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 2 new inline findings.
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot review complete: no remaining inline findings.
Resolved 2 stale NumaryBot review threads (2 fixed, 0 outdated).
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 2 new inline findings.
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #490 (comment)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/resources/ledgers/v3_spec.go`:
- Around line 147-150: Update the ReadKeySetMaxRetries validation in the
configuration conversion flow to reject all negative values while retaining the
existing int32 upper-bound check. Allow zero and positive values through
unchanged, and preserve the current error-return behavior for invalid
configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2de6f2a-68cc-4bbc-ad37-ab7611b0ef1a
⛔ Files ignored due to path filters (9)
config/crd/bases/formance.com_gatewaygrpcapis.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_gatewayhttpapis.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_ledgerconfigurations.yamlis excluded by!**/*.yamldeployment/operator/Pulumi.yamlis excluded by!**/*.yamldocs.config.yamlis excluded by!**/*.yamldocs/09-Configuration reference/settings.catalog.jsonis excluded by!**/*.jsonhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gatewaygrpcapis.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gatewayhttpapis.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_ledgerconfigurations.formance.com.yamlis excluded by!**/*.yaml
📒 Files selected for processing (22)
Dockerfileapi/formance.com/v1beta1/gatewaybackend_types.goapi/formance.com/v1beta1/gatewaygrpcapi_types.goapi/formance.com/v1beta1/gatewayhttpapi_types.goapi/formance.com/v1beta1/ledgerconfiguration_types.godeployment/operator/.gitignoredeployment/operator/helpers.godeployment/operator/main.godocs/04-Modules/03-Ledger.mddocs/09-Configuration reference/02-Custom Resource Definitions.mdinternal/resources/gateways/Caddyfile.gotplinternal/resources/ledgers/init.gointernal/resources/ledgers/v3.gointernal/resources/ledgers/v3_preview.gointernal/resources/ledgers/v3_spec.gointernal/resources/ledgers/v3_spec_test.gointernal/resources/ledgers/v3_test.gointernal/resources/ledgers/v3_tls.gointernal/resources/stacks/networkpolicies.gointernal/tests/gateway_controller_test.gointernal/tests/ledger_v3_controller_test.gointernal/tests/networkpolicy_controller_test.go
💤 Files with no reviewable changes (2)
- api/formance.com/v1beta1/gatewayhttpapi_types.go
- api/formance.com/v1beta1/gatewaygrpcapi_types.go
🚧 Files skipped from review as they are similar to previous changes (15)
- deployment/operator/.gitignore
- Dockerfile
- internal/resources/ledgers/v3_spec_test.go
- docs/04-Modules/03-Ledger.md
- api/formance.com/v1beta1/ledgerconfiguration_types.go
- api/formance.com/v1beta1/gatewaybackend_types.go
- deployment/operator/main.go
- deployment/operator/helpers.go
- internal/tests/networkpolicy_controller_test.go
- internal/resources/ledgers/v3_preview.go
- internal/tests/gateway_controller_test.go
- internal/resources/gateways/Caddyfile.gotpl
- docs/09-Configuration reference/02-Custom Resource Definitions.md
- internal/resources/ledgers/v3.go
- internal/tests/ledger_v3_controller_test.go
Comment on lines
+147
to
+150
| if configuration.ReadKeySetMaxRetries > math.MaxInt32 || configuration.ReadKeySetMaxRetries < math.MinInt32 { | ||
| return fmt.Errorf("auth read key set max retries must fit in int32, got %d", configuration.ReadKeySetMaxRetries) | ||
| } | ||
| spec.Auth.ReadKeySetMaxRetries = pointerTo(int32(configuration.ReadKeySetMaxRetries)) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C4 'ReadKeySetMaxRetries|read-key-set-max-retries' .Repository: formancehq/operator
Length of output: 9273
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C3 'AUTH_READ_KEY_SET_MAX_RETRIES|ReadKeySetMaxRetries|readKeySetMaxRetries' internal api config helmRepository: formancehq/operator
Length of output: 20514
Reject negative retry counts here. ReadKeySetMaxRetries is a retry count, and 0 already means “omit the env var / keep the default.” Allow values in the non-negative range only; < 0 forwards invalid config to the ledger.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 149-149: Narrowing a non-constant integer to a smaller fixed-width type (int8/int16/int32, uint8/uint16/uint32) can silently overflow or wrap, yielding negative or truncated values that are dangerous in size, length, or index logic. Validate the source value is within the target type's range before converting (e.g. bounds-check, or use a checked helper), and avoid narrowing untrusted or len()/parsed values.
Context: int32(configuration.ReadKeySetMaxRetries)
Note: [CWE-190] Integer Overflow or Wraparound.
(integer-overflow-narrowing-conversion-go)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/resources/ledgers/v3_spec.go` around lines 147 - 150, Update the
ReadKeySetMaxRetries validation in the configuration conversion flow to reject
all negative values while retaining the existing int32 upper-bound check. Allow
zero and positive values through unchanged, and preserve the current
error-return behavior for invalid configuration.
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #490 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #490 (comment)
| //+kubebuilder:rbac:groups=formance.com,resources=gatewaygrpcapis/finalizers,verbs=update | ||
|
|
||
| func Reconcile(ctx Context, _ *v1beta1.Stack, grpcAPI *v1beta1.GatewayGRPCAPI) error { | ||
| if grpcAPI.Spec.BackendRef != nil { |
Contributor
There was a problem hiding this comment.
🟠 [major] Missing admission guard for backendRef name collision with managed gRPC Service
When a GatewayGRPCAPI sets spec.backendRef.name to the conventional managed Service name (<spec.name>-grpc), the reconciler deletes that same Service and the generated Caddy route then points at a missing backend. The HTTP API added an equivalent admission guard for this collision, but the new gRPC API has no analogous validation. Users reusing the conventional Service name for a backendRef override can silently break gRPC routing on every reconcile cycle.
Suggestion: Add an admission validation (webhook or reconciler-level rejection) that rejects any GatewayGRPCAPI whose spec.backendRef.name matches the conventional managed Service name (<spec.name>-grpc), mirroring the guard already in place for GatewayHTTPAPI.
Member
Author
There was a problem hiding this comment.
Out of scope per maintainer direction, consistently with the earlier public GatewayGRPCAPI finding. Ledger v3 uses ledger- as its backendRef and never reuses the historical -grpc Service, so this integration is unaffected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This PR integrates the Ledger v3 Operator into the Formance Operator while preserving the historical Ledger v2 behavior.
The goal is to delegate native provisioning for Ledger versions strictly greater than
v3.0.0-alpha, allow Ledger v3 to run alongside Ledger v2 in preparation for migrations, and avoid making the Ledger Operator mandatory for stacks that do not use these features.What is included
Native Ledger v3 provisioning
v3.0.0-alpha, the Operator creates aCluster.ledger.formance.cominstead of the historical Deployments, Jobs, CronJobs, and Database resources.deployments.ledger.replicas. It defaults to3, and any positive even value is rounded up to the next odd number to preserve quorum (2 -> 3,4 -> 5).clusterID, debug mode, TLS, reserved labels, andserviceName: ledgerremain enforced by the Operator.extraEnv, Auth, and monitoring are merged field by field so that advanced options not managed by Settings are preserved, including Pyroscope, Flight Recorder,scopeMapping, andanonymousScopes.deployments.ledger.topology-spread-constraintsSetting is applied to the v3 Cluster.Shared configuration through
LedgerConfigurationLedgerConfiguration.formance.com/v1beta1CRD.spec.clusterdirectly reuses the Ledger Operator'sClusterSpec: its descriptions and Kubebuilder validations are expanded into the generated CRD without duplicating the Go type.spec.stacksfollows the Settings convention:['*']provides the global configuration, while a configuration explicitly targeting a stack takes precedence.Ledger v3 preview for preparing a v2-to-v3 migration
ledger.v3.preview-versionSetting starts an isolated v3 Cluster while fully preserving the Ledger v2 deployment and its Database./api/ledger/v3targets the v3 preview, while/api/ledger/v2and the historical routes continue to target v2.ledger.BucketServicegRPC service is routed to the v3 Cluster.TLS, Auth, and Gateway exposure
LedgerConfiguration.GatewayGRPCAPIand the associated Caddy routing alongside the HTTP rules.NetworkPolicies
7777and8888.8080, supporting mirror mode without cross-stack access.Optional dependencies and safeguards
ClusterCRD exists and that it hasget/list/watch/create/update/patch/deletepermissions before registering the Ledger v3 watcher.MigrationRequiredcondition instead of implicitly destroying data.Expected behavior
ledger.v3.preview-versionand the required CRDs runs v2 and v3 in parallel, with HTTP v2/v3 and gRPC v3 routed correctly.LedgerConfigurationexplicitly targeting a stack always takes precedence over the wildcard configuration.Validation
nix develop --command just pcgo mod tidy, linting, and generation for the affected modulesnix develop --command just testsCGO_ENABLED=0 go test ./...with Kubernetes 1.32 envtest assetsnix develop --command golangci-lint run --timeout 5m: 0 issueshelm lint ./helm/operator --stricthelm lint ./helm/crds --strictgit diff --checkA non-blocking warning remains during
just tests: the bundled Ginkgo CLI is2.27.2, while the Go packages use2.28.1.Deployment
The updated CRD chart must be deployed before this Operator version. The Ledger Operator and cert-manager are required to run Ledger v3 Clusters with TLS, but they are not required to continue operating v2 stacks.